You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Allow None for max_val_steps and max_test_steps - #160
max_val_steps and max_test_steps cap how many batches validation and test evaluate. There was no way to turn the cap off: the model plan only accepted an integer from 1 to 8000, and an unset value fell back to the run-mode default (2000 / 4000 for NORMAL and BEST). Large splits were therefore always evaluated on a sample, even though the DiskGraph datamodule already treats None as "no cap".
Change
The model plan now accepts None for both fields:
optimization:
max_val_steps: # empty / null: evaluate the whole validation splitmax_test_steps: # empty / null: evaluate the whole test split
None passes through validation, the trainers and the datamodule unchanged, and the eval loader yields every batch of the split. The docstring notes the cost: validation then runs over the whole split every epoch, and the split's logits are held in memory.
Back-compat
Plan value
Before
After
unset (???)
run-mode default
run-mode default (unchanged)
1–8000
cap at that many batches
unchanged
None / empty
rejected
no cap: whole split
0, > 8000
rejected
rejected (unchanged)
Existing plans parse and run exactly as before. A plan that sets None can only be read by a kumoapi that includes this change.
Testing
test/config/test_model_plan.py: a plan with None survives YAML and JSON round-trips and validation and stays None; an unset plan still gets 2000 / 4000.
test/train/test_diskgraph_nested_trainer.py: None reaches the datamodule unchanged.
test/train/datamodule/test_diskgraph_lightning_data.py: on a 20-row val or test split, a cap of 3 yields 3 batches (6 rows), and None yields all 10 batches (20 rows).
Suites run locally: the three files above and packages/kumo-api/test/test_json_serde.py, all passing (698 tests).
None in the model plan now means "no cap": validation and test run over
the whole split. An unset value still gets the run-mode default.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Reviewing files that changed from the base of the PR and between acc6ea5 and d6e3595.
You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.
Use the checkbox below for a quick retry:
Trigger review
No actionable comments were generated in the recent review.
Walkthrough
OptimizationPlan.max_val_steps and max_test_steps now accept None. Their existing positive-integer bounds and defaults remain unchanged. Tests cover YAML and JSON round-tripping, normal-mode validation, forwarding None limits to the datamodule, and capped or uncapped evaluation-loader counts.
Merge Risk: Minimal · previous review acc6e
Uncapped evaluation can proceed with the required stage counts; no issue identified here prevents merging after normal checks.
Pre-merge checks: 2 · 1
Failed checks: 1 inconclusive
Check name
Status
Explanation
Resolution
Change Map Design Review
The required PR conversation comment beginning with <!-- kumo-pr-change-map --> is not included in the supplied review context. Repository search found only the general review guidance in `docs/deve…
Provide the complete PR conversation Change map comment, or enable retrieval of the PR conversation. Then assess each proposed change against the available repository diff and linked problem.
Passed checks: 2 passed
Check name
Status
Explanation
Pr Size
The pull request changes 4 files. This count does not exceed 40.
Public Resource Access
The pull-request diff adds or modifies no external URL, repository location, or package source location. The changes only update type annotations, documentation, and tests.
Full details: Change Map Design Review
Explanation
The required PR conversation comment beginning with <!-- kumo-pr-change-map --> is not included in the supplied review context. Repository search found only the general review guidance in docs/development/review.md, not the PR comment. The authoritative diff is available, but this check requires assessing the proposed Change map itself, including its rationale, stopping condition, and file-specific design claims.
Resolution
Provide the complete PR conversation Change map comment, or enable retrieval of the PR conversation. Then assess each proposed change against the available repository diff and linked problem.
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
CodeRabbit Verify
Run CodeRabbit Verify to test the changes in this pull request.
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Re-reviewed d6e3595. Explicit None now reaches the existing uncapped validation/test loader behavior, while omitted settings retain 2000/4000 defaults. YAML/JSON round-trip and no-replay coverage are included, and the increased time/logit-memory cost is documented. No new correctness blocker found; this does not depend on #159 being fully streaming.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHub Actions
Inactive
Victsm
left a comment
Problem
max_val_stepsandmax_test_stepscap how many batches validation and test evaluate. There was no way to turn the cap off: the model plan only accepted an integer from 1 to 8000, and an unset value fell back to the run-mode default (2000 / 4000 forNORMALandBEST). Large splits were therefore always evaluated on a sample, even though the DiskGraph datamodule already treatsNoneas "no cap".Change
The model plan now accepts
Nonefor both fields:Nonepasses through validation, the trainers and the datamodule unchanged, and the eval loader yields every batch of the split. The docstring notes the cost: validation then runs over the whole split every epoch, and the split's logits are held in memory.Back-compat
???)1–8000None/ empty0,> 8000Existing plans parse and run exactly as before. A plan that sets
Nonecan only be read by akumoapithat includes this change.Testing
test/config/test_model_plan.py: a plan withNonesurvives YAML and JSON round-trips and validation and staysNone; an unset plan still gets 2000 / 4000.test/train/test_diskgraph_nested_trainer.py:Nonereaches the datamodule unchanged.test/train/datamodule/test_diskgraph_lightning_data.py: on a 20-row val or test split, a cap of 3 yields 3 batches (6 rows), andNoneyields all 10 batches (20 rows).packages/kumo-api/test/test_json_serde.py, all passing (698 tests).🤖 Generated with Claude Code